Skip to content

fix(mcp): add auth interceptor with channel user_id and keep header propagation to mcp tools#3294

Merged
WillemJiang merged 3 commits into
bytedance:mainfrom
zhongli-sz:fix-auth-channel-user-id-interceptor-mcp
Jun 3, 2026
Merged

fix(mcp): add auth interceptor with channel user_id and keep header propagation to mcp tools#3294
WillemJiang merged 3 commits into
bytedance:mainfrom
zhongli-sz:fix-auth-channel-user-id-interceptor-mcp

Conversation

@zhongli-sz
Copy link
Copy Markdown
Contributor

背景 / Background

在非 Web 鉴权链路(如 飞书 channel 消息)中,user_id 未正确进入运行时上下文,导致 interceptor 侧拿到默认值(default);同时 interceptor 注入的 header 在 MCP 工具调用时未透传,导致下游工具无法读取到鉴权/上下文信息。

In non-web auth flows (e.g., feishu channel messages), user_id not be correctly propagated into runtime context, causing interceptor-side fallback/default values. Also, headers injected by interceptors were not forwarded during MCP tool calls, so downstream tools could not consume auth/context headers.

本次改动 / What Changed

  1. ChannelManager 构建上下文时注入 user_id(来自 msg.user_id),避免 channel 场景丢失用户身份。
    Inject user_id from msg.user_id when building run context in ChannelManager, ensuring channel flows keep caller identity.

  2. 在网关上下文合并逻辑中,确保 user_id 写入 runtime context(不仅限于 configurable 字段)。
    Ensure user_id is merged into runtime context in gateway override merge logic (not only configurable fields).

  3. inject_authenticated_user_context 中跳过 internal 系统角色,避免内部调用覆盖/污染用户上下文。
    Skip internal system-role users in inject_authenticated_user_context to avoid overriding/polluting user context for internal calls.

  4. 在 MCP 工具调用时透传 interceptor header:将 request.headers 放入 meta.headers 传给 session.call_tool(...)
    Forward interceptor headers in MCP tool calls by passing request.headers via meta.headers to session.call_tool(...).

影响 / Impact

  • 修复 interceptor 获取到默认 user_id 的问题,用户身份在 channel 链路中可正确透传。

  • 修复 interceptor 注入 header 在 MCP 工具调用中被忽略的问题。

  • 降低鉴权上下文丢失风险,提升跨链路行为一致性。

  • Fixes default/fallback user_id issue in channel flows.

  • Fixes dropped interceptor headers during MCP tool invocation.

  • Reduces auth-context loss and improves cross-path consistency.

变更文件 / Files Changed

  • backend/app/channels/manager.py
  • backend/app/gateway/services.py
  • backend/packages/harness/deerflow/mcp/tools.py

测试建议 / Test Plan

  • [✅] Channel 消息触发一次 MCP 工具调用,确认工具侧可读取到正确 user_id(非 default)。

  • [✅] 验证 interceptor 注入 header 后,MCP 工具端可收到对应 header。

  • [✅ ] 验证 internal 系统角色请求不会覆盖普通用户上下文。

  • [✅] 回归检查常规 Web 鉴权链路不受影响。

  • [✅] Trigger an MCP tool call from a channel message and verify user_id is not default.

  • [✅] Verify interceptor-injected headers are visible in MCP tool side.

  • [✅] Verify internal system-role requests do not override user context.

  • [✅] Regression check normal web-auth flow remains unaffected.

@WillemJiang
Copy link
Copy Markdown
Collaborator

@zhongli-sz thanks for your contribution. Please add a unit test to check the user_id injection.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves identity and header propagation across non-web (channel) and MCP tool-call paths so interceptors and downstream MCP servers can consistently access caller context.

Changes:

  • Inject user_id into channel-triggered run context (from InboundMessage.user_id) to avoid default/fallback identity in channel flows.
  • Ensure user_id is merged into the gateway runtime context (config["context"]) when applying body.context overrides.
  • Forward interceptor-injected headers through MCP stdio tool calls by passing them via meta.headers, and avoid internal system-role requests overwriting user context.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File Description
backend/app/channels/manager.py Adds user_id to channel run context so channel flows preserve caller identity.
backend/app/gateway/services.py Propagates user_id into runtime context; skips stamping user_id for internal system-role users.
backend/packages/harness/deerflow/mcp/tools.py Forwards interceptor-injected headers through MCP stdio tool calls using meta.headers.

Comment thread backend/app/channels/manager.py Outdated
Comment on lines +607 to +610
{
"thread_id": thread_id,
"user_id": msg.user_id,
},
Comment on lines +154 to +155
if "user_id" in context and isinstance(runtime_context, dict):
runtime_context.setdefault("user_id", context["user_id"])
Comment thread backend/app/gateway/services.py
Comment thread backend/app/gateway/services.py Outdated
Comment on lines +171 to +172
if getattr(user, "system_role", None) == "internal":
return
Comment on lines +140 to +145
# Preserve interceptor-injected headers for stdio MCP calls by
# forwarding them through MCP call meta.
call_kwargs: dict[str, Any] = {}
if request.headers:
call_kwargs["meta"] = {"headers": dict(request.headers)}
return await session.call_tool(request.name, request.args, **call_kwargs)
@WillemJiang
Copy link
Copy Markdown
Collaborator

@zhongli-sz, thanks for your contribution. Please check out the review comment of Copilot.

@zhongli-sz
Copy link
Copy Markdown
Contributor Author

@WillemJiang Thanks for the detailed review. I’ve addressed the comments by adding regression tests for user_id propagation and MCP interceptor header forwarding, updated the docstring, and handled channel user_id safety for filesystem-scoped runtime context. Please take another look.

@WillemJiang WillemJiang added the reviewing A maintainer is reviewing this PR label May 29, 2026
@WillemJiang
Copy link
Copy Markdown
Collaborator

@zhongli-sz, here are some suggestions for this PR.

  1. Collision resistance of make_safe_user_id: Two inputs that sanitize to the same prefix but have different raw values get different 8-char digests — but SHA-1 truncated to 32 bits has ~1M-entry birthday bound. If this ever scopes storage buckets, consider a longer digest (e.g., 12–16 hex chars) or document the expected scale. For current IM channel use (millions, not billions), 8 hex chars is likely fine.
  2. request.headers type safety (tools.py:142–143): dict(request.headers) could fail if request.headers is None rather than an empty dict. The if request.headers: guard handles the falsy case, but if headers could be a Mapping that's truthy but not dict-constructible, this would be an edge case. Worth verifying the MCPToolCallRequest contract guarantees a dict or None.
  3. system_role attribute coupling (services.py:178): getattr(user, "system_role", None) == "internal" is a string comparison against a magic value. If system_role gains more values or is refactored, this check could silently stop matching. A class constant or enum would be more maintainable, though this is minor given the current scope.
  4. Test for _resolve_run_params with empty/None msg.user_id: The new tests cover safe and unsafe IDs but not the case where msg.user_id is None or empty string. The if msg.user_id: guard skips injection, but it'd be good to have an explicit test confirming user_id and channel_user_id are absent from the context in that case.

@WillemJiang WillemJiang added the question Further information is requested label May 29, 2026
@zhongli-sz
Copy link
Copy Markdown
Contributor Author

@WillemJiang Thanks for the detailed review — I’ve addressed all follow-up points:

make_safe_user_id collision resistance
Increased the digest suffix from 12 to 16 hex chars to further reduce collision probability for sanitized IDs used in user-scoped buckets.

request.headers type safety in MCP tool forwarding
Added a defensive Mapping type check before converting headers for meta.headers.
If a truthy non-mapping value is provided, it is now safely ignored with a warning (no exception).

system_role magic string coupling
Replaced the inline "internal" string comparison with a shared constant (INTERNAL_SYSTEM_ROLE) from internal_auth to avoid string drift.

_resolve_run_params empty/None msg.user_id case
Added explicit tests for "" and None, verifying that both user_id and channel_user_id are not injected into run context in those cases.

I also reran the related test suite for this PR scope (channels/gateway/mcp/paths), and everything is passing.

for1314ai and others added 3 commits June 1, 2026 10:29
…n tests

Normalize external channel user ids into filesystem-safe runtime context while preserving raw channel_user_id, and document gateway user_id propagation semantics. Add regression coverage for channel user_id context mapping, gateway user_id precedence/internal-role behavior, and MCP interceptor header forwarding via meta.headers.

Co-authored-by: Cursor <cursoragent@cursor.com>
Increase sanitized user-id digest suffix to 16 hex chars, replace internal system role magic string with a shared constant, and harden MCP header forwarding with Mapping type checks. Add regression tests for empty channel user_id handling, unsupported header types, and updated digest length behavior.

Co-authored-by: Cursor <cursoragent@cursor.com>
@zhongli-sz zhongli-sz force-pushed the fix-auth-channel-user-id-interceptor-mcp branch from fed2f30 to 8f49b5c Compare June 1, 2026 02:29
@WillemJiang WillemJiang merged commit 3ae82dc into bytedance:main Jun 3, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

question Further information is requested reviewing A maintainer is reviewing this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants